feat: add sharing debug information option to main settings screen - WPB-24712#4677
feat: add sharing debug information option to main settings screen - WPB-24712#4677
Conversation
| present(deleteDatabaseConfirmationAlert, animated: true) | ||
| } | ||
|
|
||
| func mailComposeController( |
There was a problem hiding this comment.
todo: check this change
Test Results – WireUIAll 1 files 41 suites 1m 11s ⏱️ For more details on these failures, see this check. Results for commit 86bd0f7. ♻️ This comment has been updated with latest results. |
Test Results – WireFoundationAll47 tests 47 ✅ 10s ⏱️ Results for commit 86bd0f7. ♻️ This comment has been updated with latest results. |
Test Results – Wire-iOS1 881 tests 1 854 ✅ 2m 11s ⏱️ Results for commit 86bd0f7. ♻️ This comment has been updated with latest results. |
Test Results – WireLoggingAll4 tests 4 ✅ 0s ⏱️ Results for commit 86bd0f7. ♻️ This comment has been updated with latest results. |
Test Results – WireUtilities280 tests 280 ✅ 6s ⏱️ Results for commit 86bd0f7. ♻️ This comment has been updated with latest results. |
| let isOn = keyAndValue[1] == "true" | ||
| flag.enable(isOn) | ||
| } | ||
| #if DEBUG |
There was a problem hiding this comment.
set developerFlags passed from XCUITest
| private var defaultRecordMode: SnapshotTestingConfiguration.Record? { | ||
| let ci = ProcessInfo.processInfo.environment["CI"] | ||
| return (ci == nil || ci?.isEmpty == true) ? .missing : .never | ||
| if let value = ProcessInfo.processInfo.environment["SNAPSHOT_TESTING_RECORD"], |
There was a problem hiding this comment.
now, it reads the env variable from the testplan first so if you need to record snapshots you can just modify the env value to failed and not remove the file manually
netbe
changed the title
|
samwyndham
left a comment
samwyndham
left a comment
There was a problem hiding this comment.
Nice work. I left some comments before approving
| "enabled" : false, | ||
| "key" : "SNAPSHOT_TESTING_RECORD", | ||
| "value" : "failed" | ||
| }, |
There was a problem hiding this comment.
question: Interesting. Does this mean that if we set it to true it will recreate the failing snapshots for us?
There was a problem hiding this comment.
question: OK so the change here is the color of the tab bar right? Was this indended?
| // MARK: - setUp / tearDown | ||
|
|
||
| override func setUp() async throws { | ||
| tempDirectory = URL(fileURLWithPath: NSTemporaryDirectory()) |
There was a problem hiding this comment.
nit pick: You can also do: URL.temporaryDirectory
| /// Style iOS & Figma: ? | ||
| case largeTitle | ||
|
|
||
| /// Style iOS & Figma: Title 3 |
There was a problem hiding this comment.
praise: Adding comments here. I guess at some point we should rename things to match Figma design system if it is up-to-date
There was a problem hiding this comment.
thought (out of scope): This test looks weird. I hope our tab bar never looks like that in reality
| /// On XCUITests, shake gesture is not available. | ||
| public var useTripleTapForShakeGesture = true | ||
|
|
||
| /// Developer flags to apply at launch, keyed by `DeveloperFlag.rawValue`. |
There was a problem hiding this comment.
thought: I wonder if we should move DeveloperFlag to WirePrimitives so that we can pass them directly instead of their string values.
|
|
||
| import XCTest | ||
|
|
||
| final class ShareDebugReportTests: WireUITestCase { |
|
|
||
| /// When `true`, a triple-tap on the app window triggers the same action as the shake gesture. | ||
| /// On XCUITests, shake gesture is not available. | ||
| public var useTripleTapForShakeGesture = true |
There was a problem hiding this comment.
suggestion: Have this disabled by default and only enable it in the setup of the UITests that use it.
issue(maybe): I was looking at UITestConfig yesterday and realized that we should change something... Currently if UITestConfig.environment returns a default if it is not set in the environment. I think it should return nil. I think the in the current implementation any kind of debug build will have useTripleTapForShakeGesture, not just UI tests
| } | ||
| } | ||
|
|
||
| func simulateShakeGesture() { |
There was a problem hiding this comment.
suggestion: Extend and apply to XCUIApplication instead of WireUITestCase
| } | ||
|
|
||
| func invoke() async throws -> URL { | ||
| try await withCheckedThrowingContinuation { [logsProvider, selfUserID] continuation in |
There was a problem hiding this comment.
question: Why is this being doing using GCD?
2 participants
Issue
This PR reworks how sharing debug information is presented to users. It's now accessible :
Default behaviour: it still shows the DeveloperTools screen. If the Developer flag is on, it shows the action sheet. Shake again to present the DeveloperTools.
On production it only shows the action sheet.
Testing
Checklist
[WPB-XXX].UI accessibility checklist
If your PR includes UI changes, please utilize this checklist: